Skip to content

feat(analyzer): opt-in TRUST_INTURN_RECOVERY flag for hook-retry denials - #348

Open
nitrocode wants to merge 1 commit into
frankbria:mainfrom
nitrocode:fix/trust-inturn-recovery-flag
Open

feat(analyzer): opt-in TRUST_INTURN_RECOVERY flag for hook-retry denials#348
nitrocode wants to merge 1 commit into
frankbria:mainfrom
nitrocode:fix/trust-inturn-recovery-flag

Conversation

@nitrocode

Copy link
Copy Markdown

Problem

should_exit_gracefully halts the next loop when the previous transcript's permission_denials array is non-empty. This is a false positive when a PreToolUse hook denies a Write/Edit tool call with a corrective message and Claude retries the tool call successfully within the same turn — the turn ends with is_error: false and stop_reason: end_turn, work completes, but the next loop halts on permission_denied anyway. Issue #243's exception only covers Bash compound-command denials; hook denials on file-mutation tools have no equivalent carve-out.

Repro

  1. Configure a project-level PreToolUse hook that denies Write / Edit when the input matches some rule (e.g. a style hook that rewrites tool_input and returns a corrective deny decision).
  2. Run ralph on a task that produces a matching Write/Edit.
  3. Claude receives the deny + correction, retries with the fix, succeeds. Turn ends cleanly (is_error: false, stop_reason: end_turn).
  4. On the next loop iteration, should_exit_gracefully halts with Permission denied for N command(s): Write, Edit ... Update ALLOWED_TOOLS in .ralphrc, even though nothing is actually blocked and the previous loop's work already landed.

Observed 3 times in one session on this exact pattern.

Fix (opt-in, off by default)

Following the guidance in #264's closing comment — that a broader denial-recovery fix "would be welcome" but "gated behind a config flag, so the default keeps the strict #101 halt" — this PR adds an opt-in TRUST_INTURN_RECOVERY flag (default false) to .ralphrc/templates/ralphrc.template. When true, and every denial in a completed turn is for Write / Edit / NotebookEdit / MultiEdit AND the CLI reports is_error: false + stop_reason: end_turn, should_exit_gracefully logs an advisory and continues instead of halting. Structurally parallel to the Issue #243 compound-command exception (#268).

Default behavior (flag unset or false) is unchanged. All 1007 pre-existing unit tests pass (2 unrelated test_sandbox_e2b.bats failures reproduce on a clean checkout of main too — local Python environment issue on my machine, not caused by this change).

Residual limitation (documented in the ralphrc template with a WARNING: comment, matching the existing CB_AUTO_RESET convention)

is_error: false and stop_reason: end_turn are outcome signals ("the turn didn't crash after the denial") not authorization signals ("the specific denied call was resolved"). A hook that denies a Write to a sensitive path (e.g. .ralphrc itself) which the agent then abandons and moves past would also produce a clean turn end. Enabling TRUST_INTURN_RECOVERY weakens the Issue #101 silent-loop guard for this class of case, which is why it defaults to false and requires explicit opt-in.

Stronger signals (walking assistant messages for a subsequent successful tool_use of the same tool with matching input) were considered and left out to keep this PR minimally scoped, consistent with #268's surgical style.

Verification

  • tests/unit/test_hook_retry_recovery.bats (new, 9 tests): flag gating (unset/false/true), coverage of all four file-mutation tools, is_error: true still halts, Bash denials stay out of scope, mixed denials still halt, and two end-to-end tests that exercise the full analyze_response path (not just parse_json_response) to confirm the new fields actually propagate into the .response_analysis file that should_exit_gracefully reads from .analysis.*parse_json_response's result is re-extracted and re-packaged inside analyze_response, so a wiring gap there would otherwise pass the parse-level tests while still no-op'ing in production.
  • tests/unit/test_compound_command_detection.bats (existing, 27 tests): all pass unchanged, confirming the Issue Permission denied but already configured in ALLOWED_TOOLS #243 path is untouched.
  • Full tests/unit/*.bats suite: 1007/1009 pass; the 2 failures are pre-existing and unrelated (see above).

References

A PreToolUse hook can deny a Write/Edit/NotebookEdit/MultiEdit tool call
with a corrective message. Claude Code retries the tool call within the
same turn and succeeds (is_error=false, stop_reason=end_turn), but the
next loop halts anyway because the denial still appears in the previous
transcript. Issue frankbria#243's exception only covers Bash compound-command
denials with an already-permitted base; hook denials on file-mutation
tools have no equivalent carve-out.

Adds an opt-in TRUST_INTURN_RECOVERY flag (default false, matching the
approach suggested in PR frankbria#264's closing comment) to .ralphrc. When true,
and every denial in a completed turn is for a file-mutation tool AND the
turn ended cleanly, should_exit_gracefully logs an advisory and continues
instead of halting. Default behavior is unchanged.

is_error/stop_reason are outcome signals, not authorization signals like
Issue frankbria#243's ALLOWED_TOOLS-coverage check, so this stays opt-in and off
by default: a clean turn end does not prove the specific denied write
was resolved, only that the turn did not crash afterward.

Refs: frankbria#264
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant